Rebase - #107

uploadstblogs/src/file_operations.c:563

• add_timestamp_to_files_uploadlogsnow() calls strftime() without checking its return value. If strftime() returns 0, the timestamp buffer contents are unspecified, and strlen(timestamp)/strncmp() logic can end up skipping all files (or behaving unpredictably). Mirror add_timestamp_to_files(): check the strftime() result and return an error (or fallback) when formatting fails.

uploadstblogs/src/file_operations.c:563

• In add_timestamp_to_files_uploadlogsnow(), the return value of strftime() is not checked. If it returns 0 (e.g., unexpected format failure), timestamp will be empty and the subsequent prefix checks/renames will behave incorrectly. Capture the return value and treat 0 as an error (log and return -1) before using timestamp/g_timestamp_prefix.

uploadstblogs/unittest/Makefile.am:48

• archive_manager_gtest.cpp uses dlsym/RTLD_NEXT, which typically requires linking with -ldl on glibc. COMMON_LDADD currently doesn’t include -ldl, so archive_manager_gtest may fail to link.
  uploadstblogs/src/cleanup_handler.c:255
• In cleanup_old_archives(), the subdirectory path is built with snprintf() but the return value isn’t checked. If the path is truncated, the recursion will run on an incorrect path (and could delete the wrong files).
  uploadstblogs/src/strategy_handler.c:76
• cleanup_old_archives() return value is ignored. If cleanup fails (e.g., ctx->log_path missing/permission denied), the workflow continues silently, which makes failures hard to diagnose.
  uploadstblogs/unittest/archive_manager_gtest.cpp:707
• This main() sets GTest JSON output to /tmp/Gtest_Report/, but it never ensures the directory exists. Also, buffer is unused and can trigger warnings under stricter builds.
  backup_logs/src/config_manager.c:40
• config_load() uses 32-byte temporary buffers for LOG_PATH and APP_PERSISTENT_PATH. These paths can easily exceed 31 characters, causing truncation and incorrect derived paths.

int config_load(backup_config_t* config) {
    char log_path_buf[32] = {0};
    char hdd_enabled_buf[32] = {0};
    char app_persistent_path_buf[32] = {0};
    

uploadstblogs/docs/lld/uploadSTBLogs_LLD.md:202

• The LLD claims the SHA256-to-hex conversion uses a nibble lookup table to avoid per-byte snprintf overhead, but the actual implementation in md5_utils.c uses snprintf in a loop. The doc should match the implementation (or the implementation should be updated).
  LICENSE:224
• The repository LICENSE file is Apache-2.0, but a generic MIT template (with placeholder "YEAR COPYRIGHT HOLDER") has been appended. This is not a valid license notice and can create legal ambiguity; third-party MIT components should be documented via NOTICE (or a dedicated third-party licenses file), not by appending a template to LICENSE.

                            MIT License

Copyright (c) YEAR COPYRIGHT HOLDER

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.

test/functional-tests/tests/test_uploadstblogs_sync_gates.py:43

• This test uses subprocess.run(...), but the module only imports subprocess as sp. As written it will raise NameError before any assertions run.
  test/functional-tests/tests/test_uploadstblogs_sync_gates.py:116
• This test expects the literal log line "bacukup_logs sentinel detected. Proceeding." but that string is not present anywhere in uploadstblogs sources, so the assertion will always fail. Either update the production log message or change the test to validate behavior via existing logs/phase progress.
  configure.ac:137
• backup_logs is added as a new subproject, and backup_logs/src/sys_integration.c links against libsystemd (-lsystemd), but configure.ac doesn’t check for libsystemd headers/libs or provide a conditional build. This can break cross-compilation/builds where systemd isn’t available.
  uploadstblogs/src/event_manager.c:191
• emit_upload_aborted() is called when uploads are intentionally skipped (e.g., RFC/reboot-reason gating), but it now sends LOG_UPLOAD_FAILED. That will incorrectly report a failure (and may trigger error handling) for a deliberate abort/skip. It should emit LOG_UPLOAD_ABORTED (as before) to preserve semantics.

test/functional-tests/tests/test_uploadstblogs_sync_gates.py:42

• This test file imports subprocess as sp, but calls subprocess.run(...) which is undefined and will raise a NameError at runtime.
  test/functional-tests/tests/test_uploadstblogs_sync_gates.py:105
• subprocess.run(...) is used again here, but subprocess is not imported (only import subprocess as sp). This will fail the test run.
  test/functional-tests/tests/test_uploadstblogs_sync_gates.py:115
• The expected log string is misspelled as "bacukup_logs..." but the implementation logs "backup_logs sentinel detected. Proceeding." (uploadstblogs/src/strategies.c:899). As written, this assertion will always fail.
  test/functional-tests/tests/test_uploadstblogs_sync_gate_backup_logs.py:124
• The expected log string is misspelled as "bacukup_logs..." but the implementation logs "backup_logs sentinel detected. Proceeding." (uploadstblogs/src/strategies.c:899). This assertion will fail even when the gate passes.
  test/functional-tests/tests/test_uploadstblogs_sync_gate_backup_logs.py:166
• The expected log string is misspelled as "bacukup_logs..." but the implementation logs "backup_logs sentinel detected. Proceeding." (uploadstblogs/src/strategies.c:899). This check should use the correct string.
  uploadstblogs/src/event_manager.c:191
• emit_upload_aborted() now emits LOG_UPLOAD_FAILED, but the event mapping explicitly supports LOG_UPLOAD_ABORTED (and callers use this function for abort flows). This makes aborts indistinguishable from failures for IARM consumers, and leaves the LOG_UPLOAD_ABORTED mapping unused.
  uploadstblogs/src/cleanup_handler.c:255
• When recursing into a subdirectory, the snprintf() result is not checked. If the path is truncated, cleanup_old_archives() may recurse into an unintended path (or fail to clean the intended one). The regular-file branch already guards against this; the directory branch should too.
  uploadstblogs/unittest/archive_manager_gtest.cpp:703
• GTest JSON output is directed to /tmp/Gtest_Report/, but that directory is never created. When fopen() is delegated to the real libc implementation, report creation will fail if the directory is missing.
  usbLogUpload/unittest/usb_log_main_gtest.cpp:55
• This test is labeled "valid input" but it no longer asserts success; it only asserts the function did not return USB_NOT_MOUNTED. Renaming the test (and its brief) would avoid overstating what it verifies.
  LICENSE:207
• The MIT license stanza still contains the template placeholder "YEAR COPYRIGHT HOLDER", which makes the licensing text incomplete/incorrect.

                            MIT License

Copyright (c) YEAR COPYRIGHT HOLDER

uploadstblogs/docs/lld/uploadSTBLogs_LLD.md:202

• The LLD claims the SHA256 hex conversion uses a nibble lookup table, but the current implementation in uploadstblogs/src/md5_utils.c uses per-byte snprintf(). Either update the implementation or adjust this note to match reality.
  backup_logs/src/config_manager.c:96
• HDD_ENABLED parsing currently treats any value other than literal "false" as enabled (e.g., "invalid" or empty string becomes true). That contradicts the expected boolean semantics and will select the wrong backup strategy.

    /* Check HDD_ENABLED like shell script */
    if (getDevicePropertyData("HDD_ENABLED", hdd_enabled_buf, sizeof(hdd_enabled_buf)) == UTILS_SUCCESS) {
        config->hdd_enabled = (strcmp(hdd_enabled_buf, "false") != 0);
        RDK_LOG(RDK_LOG_INFO, LOG_BACKUP_LOGS, "HDD_ENABLED loaded from properties: %s (evaluated to %s)\n", 
                hdd_enabled_buf, config->hdd_enabled ? "true" : "false");
    } else {
        config->hdd_enabled = false;  /* Default to false if not found */
        RDK_LOG(RDK_LOG_WARN, LOG_BACKUP_LOGS, "HDD_ENABLED not found in properties, using default: false\n");
    }

uploadstblogs/src/event_manager.c:190

• emit_upload_aborted() is emitting LOG_UPLOAD_FAILED even though LOG_UPLOAD_ABORTED is defined; this makes an intentional abort indistinguishable from a real failure (and conflicts with the function name).
  uploadstblogs/src/event_manager.c:435
• emit_folder_missing_error() is used when PREV_LOG_PATH is missing (uploadstblogs/src/validation.c notes this should map to MAINT_LOGUPLOAD_ERROR). Emitting MAINT_LOGUPLOAD_COMPLETE here reports success for an error condition.
  uploadstblogs/src/file_operations.c:567
• add_timestamp_to_files_uploadlogsnow() should check strftime() return value (it can return 0 on buffer too small or other failure), similar to add_timestamp_to_files().
  uploadstblogs/docs/lld/uploadSTBLogs_LLD.md:201
• The LLD states the SHA256 hex conversion uses a nibble lookup table to avoid per-byte snprintf overhead, but the implementation in uploadstblogs/src/md5_utils.c currently uses snprintf per byte. This is a doc/implementation mismatch.
  test/functional-tests/tests/test_uploadstblogs_sync_gates.py:42
• This test uses subprocess.run() but only imports subprocess as sp; as written it will raise NameError and fail immediately.
  LICENSE:206
• The MIT license text includes the placeholder "YEAR COPYRIGHT HOLDER", which should be replaced with the actual copyright holder/year to avoid an invalid/ambiguous LICENSE file.

Copyright (c) YEAR COPYRIGHT HOLDER

uploadstblogs/unittest/archive_manager_gtest.cpp:703

• archive_manager_gtest.cpp main() declares and memset()s a 'buffer' that is never used. This can trigger -Wunused-variable in some toolchains and breaks -Werror builds.
  backup_logs/src/config_manager.c:39
• The path buffers are only 32 bytes, which can truncate valid LOG_PATH/APP_PERSISTENT_PATH values and lead to incorrect derived paths. Use PATH_MAX (or sizeof(destination)) for these temporary buffers.

    char log_path_buf[32] = {0};
    char hdd_enabled_buf[32] = {0};
    char app_persistent_path_buf[32] = {0};

backup_logs/src/config_manager.c:92

• HDD_ENABLED parsing treats any value other than literal "false" as true (including "invalid"), but the added feature specs expect invalid values to default to false with a warning.

    if (getDevicePropertyData("HDD_ENABLED", hdd_enabled_buf, sizeof(hdd_enabled_buf)) == UTILS_SUCCESS) {
        config->hdd_enabled = (strcmp(hdd_enabled_buf, "false") != 0);
        RDK_LOG(RDK_LOG_INFO, LOG_BACKUP_LOGS, "HDD_ENABLED loaded from properties: %s (evaluated to %s)\n", 
                hdd_enabled_buf, config->hdd_enabled ? "true" : "false");

uploadstblogs/src/Makefile.am:23

• This change adds -lparsejson to the link line, but configure.ac does not check for the presence of the parsejson library/header. This can lead to configure succeeding but builds failing later on some targets.
  configure.ac:136
• backup_logs is now part of the build (AC_CONFIG_FILES includes backup_logs/Makefile), but configure.ac does not check for libsystemd headers/libs even though backup_logs links -lsystemd and includes <systemd/sd-daemon.h>. This can produce late build failures on platforms without systemd dev packages.